fix(operator): back off exponentially on 429 and fail liveness when the loop stalls - #258
Conversation
SonarQube — aucune nouvelle issueComparaison entre le projet bac à sable de cette PR et la branche par défaut : SonarQube Community n'analyse pas les PR, ce delta est calculé côté CI. Détail |
## [5.2.9] - 2026-09-07 ### Bug Fixes - fix(operator): back off exponentially on 429 and fail liveness when the loop stalls (#258)
![ferrfleet[bot]](https://avatars.githubusercontent.com/in/4354888?s=60&v=4){kind=small}
There was a problem hiding this comment.
The backoff change is sound: the arithmetic in the description holds, per-key state with forget on success is the right shape, and the test that asserts the old 20s wait was saturating before checking the new one is a good way to stop a regression.
Two blocking findings, both in the liveness check, both cases where it fails a healthy operator and restart-loops it. This is already merged, so they need a follow-up.
- Non-leader replicas never beat, so every standby is killed once per
stallThreshold. Multi-replica is documented as supported invalues.yaml. - The check treats "a FerrVaultSecret exists" as implying a reconcile within 15m, but secrets requeue at
refreshInterval(1h default). A cluster with secrets and no connection restart-loops.
Nothing to add on the rate-limit work.
| return now.Sub(h.last) | ||
| } | ||
|
|
||
| func StallChecker(c client.Client, hb *Heartbeat, threshold time.Duration) healthz.Checker { |
There was a problem hiding this comment.
Blocking: on a non-leader replica this check can never pass.
values.yaml documents multi-replica as a supported configuration ("With leader election enabled, running more than one replica is safe (only the elected leader reconciles)"). Controllers are leader-election runnables and only start once the lease is won. The health probe server is not, and serves from process start. So on a standby replica Reconcile never runs, nothing ever calls Beat, and last stays at the NewHeartbeat(time.Now()) from cmd/main.go.
Once stallThreshold elapses and any FerrVaultConnection exists, /healthz returns 500 and the kubelet restarts the standby container, roughly every 15m, indefinitely. That is worse than no HA: the replica that exists to take over sits in CrashLoopBackOff, and it re-enters the lease race on every restart.
The fix spans this file and cmd/main.go, so no suggestion block: pass mgr.Elected() through to StallChecker and return nil while that channel is still open. A replica that was never elected has no reconciles to be missing, which is the same reasoning that already exempts an idle cluster.
| func hasWatchedResources(ctx context.Context, c client.Client) (bool, error) { | ||
| var conns fvv1alpha1.FerrVaultConnectionList | ||
| if err := c.List(ctx, &conns, client.Limit(1)); err != nil { | ||
| return false, err | ||
| } | ||
| if len(conns.Items) > 0 { | ||
| return true, nil | ||
| } | ||
| var secrets fvv1alpha1.FerrVaultSecretList | ||
| if err := c.List(ctx, &secrets, client.Limit(1)); err != nil { | ||
| return false, err | ||
| } | ||
| return len(secrets.Items) > 0, nil | ||
| } |
There was a problem hiding this comment.
Blocking: "a FerrVaultSecret exists" does not imply a reconcile inside the threshold.
The justification in the description holds for connections: connectionProbeInterval is 10m, under the 15m default, and the reconcile tail always returns RequeueAfter: connectionProbeInterval. It does not hold for secrets. A secret reconcile requeues at r.refreshInterval(&cr), which falls back to --default-refresh-interval, chart default 1h. failReady uses that same interval.
So a cluster with at least one FerrVaultSecret and zero FerrVaultConnections reconciles each secret once, takes ConnectionNotFound, requeues an hour out, and is killed by liveness 15m later. Restart, one reconcile per secret, killed again. That state is reachable by applying a FerrVaultSecret before its connection or by typoing connectionRef.name, and the resulting restart loop means the operator is not running to pick up the correction.
Narrowing the check to connections keeps the invariant the threshold was chosen against. A cluster holding only secrets is quiet by design, and nothing there could sync anyway:
| func hasWatchedResources(ctx context.Context, c client.Client) (bool, error) { | |
| var conns fvv1alpha1.FerrVaultConnectionList | |
| if err := c.List(ctx, &conns, client.Limit(1)); err != nil { | |
| return false, err | |
| } | |
| if len(conns.Items) > 0 { | |
| return true, nil | |
| } | |
| var secrets fvv1alpha1.FerrVaultSecretList | |
| if err := c.List(ctx, &secrets, client.Limit(1)); err != nil { | |
| return false, err | |
| } | |
| return len(secrets.Items) > 0, nil | |
| } | |
| // Only a FerrVaultConnection guarantees a reconcile inside the stall | |
| // threshold: it re-probes every connectionProbeInterval. A FerrVaultSecret | |
| // requeues at its refreshInterval, which falls back to an hour, so a cluster | |
| // holding secrets and no connection is quiet by design rather than stalled. | |
| func hasWatchedResources(ctx context.Context, c client.Client) (bool, error) { | |
| var conns fvv1alpha1.FerrVaultConnectionList | |
| if err := c.List(ctx, &conns, client.Limit(1)); err != nil { | |
| return false, err | |
| } | |
| return len(conns.Items) > 0, nil | |
| } |
1 participant
Addresses FerrLabs/FerrVault-Cloud#845. Closing keywords do not cross repositories, so that issue needs closing by hand.
The issue names two defects. One I could prove from the code and have fixed; the other I could not diagnose, and this contains it rather than curing it. Both are called out below for what they are.
The 429 storm is self-sustaining, and the code says why
rateLimitRequeuewas a fixed 20s, and the comment above it stated the assumption that fails: "the API's bucket refills at one token per second, so a short wait is enough for a handful of resources to get through". It is, for a handful.N resources retrying every fixed D seconds offer N/D requests per second against a bucket that refills at 1/s per caller token. At D=20s, twenty-one resources already offer more than the refill rate, so the bucket never recovers and every resource keeps taking 429s. The issue reports 29 secrets on one token: 1.45/s offered against 1/s. That is not a burst that clears, it is a livelock, which matches "22 in 429, 1 synced out of 29" still reading 1/29 two minutes later.
The wait now doubles from 5s to a 5m ceiling, per resource, with jitter over the lower half of each delay so the herd stops retrying in lockstep. The offered rate crosses under the refill rate within about four rounds regardless of N, and a success forgets the counter. Both controllers use it; the connection probe had the same fixed wait.
The loop stalling: contained, not diagnosed
I could not find the cause by reading, and I will not guess in a commit message. What I checked and ruled out: the HTTP client already caps every call at 10s, its retry loop is bounded, honours context cancellation and returns 4xx immediately, so a request cannot hang a worker forever.
What I did not rule out, and is worth someone's attention with the cluster in hand:
MaxConcurrentReconcilesis unset, so both controllers run one reconcile at a time and any single slow path serialises everything behind it.So this makes the stall visible instead of silent.
/healthzgains areconcile-progresscheck that fails when no reconcile has completed for--stall-threshold(default 15m) and at least one FerrVaultConnection or FerrVaultSecret exists. The liveness probe already points at/healthz, so a stalled operator is restarted instead of sitting Ready with nothing happening.Two deliberate choices against a restart loop, which would be worse than the bug:
One deviation from the issue, which suggests keying on the last successful reconcile: the heartbeat beats when a reconcile completes, success or not. Keying on success would restart the pod whenever the FerrVault backend is legitimately down for longer than the threshold, which fixes nothing and loses the operator too. A loop that keeps failing fast is still a running loop; the failure mode here is that nothing runs at all.
The default of 15m sits above the 10m connection probe interval, which is the shortest reconcile a healthy operator is guaranteed to perform whenever any resource exists.
One claim from the issue I could not reproduce
The issue reads the four
TokenUnreadableconnections as having drained the budget for the twenty healthy secrets, and suggests not spending a rate-limit token on a request that fails before it is sent. That already holds: in both controllers an unreadable token secret short-circuits before any client is built, so no request leaves the operator. The amplification I can account for is the retry storm above, which is fixed here. If the 1:5 ratio has another mechanism behind it, it is still unexplained.Verification
go build,go vetandgo test ./...are green. New tests cover the parts that can actually regress:forgetresetting to base, and keys staying independentThe chart change is not covered locally, helm is not installed here;
e2e.ymlinstalls the chart, so a broken template fails there.